Conversation
RangerMauve
left a comment
There was a problem hiding this comment.
This is looking great so far, TY!
Just left some comments on how we can reduce the surface changed further and make the testing for both paths more uniform.
| const Hyperbee = require('hyperbee') | ||
| const b4a = require('b4a') | ||
|
|
||
| module.exports = class Autodeebee { |
There was a problem hiding this comment.
Do you think you could publish this as a separate module which we could link to in the README for folks to test and include in the devDependencies for the tests?
index.js
Outdated
| return ensureComparable(docValue) <= ensureComparable(queryValue) | ||
| } | ||
|
|
||
| function compareNe (docValue, queryValue) { |
There was a problem hiding this comment.
Could this feature be split into a separate PR?
| @@ -0,0 +1,1074 @@ | |||
| const test = require('tape') | |||
There was a problem hiding this comment.
Could you refactor test.js so that we can have all the tests take a getBee parameter which will be run with both the regular hyperbee and the autobee?
This way we don't need to duplicate all the test code and can make sure that new tests cover both sides.
e.g.
makeTests(getBee, 'Hyperbee')
makeTests(getAutoBee, 'Autobee')
function makeTests(getBee, type) {
test(`${type} Create a document in a collection`, async (t) => {
const db = new DB(getBee())
.... etc
}|
i can't wait till this is merged. i wanna get started and work with a multiwriter hyperbeedeebee asap. hope this pr is not forgotten about or abandoned. |
|
Hello, |
glad this is still being worked on |
Hello Mauve,
Thanks a lot for the comment. I didn't thought it was possible to reach the same API, because of the sub function.
I gave it a go and it worked!
All the changes are now in the
autodeebee.jsfile (ex SimpleAutobee)Still got more tests to add specifically to Autobase. I tried it in my app and it works so it sounds good.
Cheers,
Jamps